-
Notifications
You must be signed in to change notification settings - Fork 0
feat: Implement resource requirements for components #194
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Co-authored-by: toby-coleman <[email protected]>
Co-authored-by: toby-coleman <[email protected]>
Co-authored-by: toby-coleman <[email protected]>
Co-authored-by: toby-coleman <[email protected]>
| """Parse a resource value from string or number. | ||
|
|
||
| Supports: | ||
| - Direct numerical values: 1, 0.5, 2.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot Can you make this more general? For example by using/supporting any of the following suffixes:
suffixes = {
'n': 1e-9, 'u': 1e-6, 'm': 1e-3, # Decimal SI
'k': 1e3, 'M': 1e6, 'G': 1e9, 'T': 1e12, 'P': 1e15, 'E': 1e18,
'Ki': 1024, 'Mi': 1024**2, 'Gi': 1024**3, 'Ti': 1024**4, 'Pi': 1024**5, 'Ei': 1024**6
}There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done. Extended to support all decimal SI prefixes (n, u, m, k, M, G, T, P, E) and binary prefixes (Ki, Mi, Gi, Ti, Pi, Ei) in commit 9515154.
|
|
||
| cpu: float = 0.001 | ||
| gpu: float = 0 | ||
| memory: float = 0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Memory should be specified as an integer number of bytes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done. Changed memory field type to int and added separate validator to cast to integer in commit 9515154.
| initial_values: _t.NotRequired[dict[str, _t.Any] | None] | ||
| parameters: _t.NotRequired[dict[str, _t.Any] | None] | ||
| constraints: _t.NotRequired[dict[str, _t.Any] | None] | ||
| resources: _t.NotRequired["Resource | None"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| resources: _t.NotRequired["Resource | None"] | |
| resources: _t.NotRequired[Resource | None] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done in commit 9515154.
plugboard/process/ray_process.py
Outdated
| ).remote(**args) | ||
|
|
||
| # Get resource requirements from component | ||
| from plugboard.schemas import Resource |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Move this import to the top
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done. Moved Resource import to top of file in commit 9515154.
plugboard/tune/tune.py
Outdated
| Returns: | ||
| List of resource bundles for Ray placement group. | ||
| """ | ||
| from plugboard.schemas import Resource |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Move this import to the top
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done. Moved Resource import to top of file in commit 9515154.
toby-coleman
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot see comments
…r, move imports to top Co-authored-by: toby-coleman <[email protected]>
Summary
Adds resource specification support for components to enable Ray actor resource allocation and Tuner placement group optimization.
Changes
Core Schema
ResourcePydantic class inplugboard_schemas.componentwith fields:cpu,gpu,memory(integer bytes),resources(custom dict)n,u,m,k,M,G,T,P,E(e.g.,"250m"→ 0.25,"5k"→ 5000)Ki,Mi,Gi,Ti,Pi,Ei(e.g.,"10Mi"→ 10,485,760 bytes)cpu=0.001, others zeroto_ray_options()for Ray actor configurationComponent Integration
Component.__init__()with optionalresourcesparameterComponentArgsDict,ComponentArgsSpecfor YAML supportRay Execution
RayProcess._create_component_actor()to apply resource requirements viaray.remote(**ray_options)Resource()when unspecifiedTuner Optimization
Tuner._calculate_placement_bundles()to aggregate component resourcesPlacementGroupFactorywith tuner overhead (0.5 CPU) + component bundleDocumentation
docs/examples/tutorials/running-in-parallel.mdreferencing resource requirements exampleUsage Example
Tests
Original prompt
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.